Add reconstruction unit tests#128
Add reconstruction unit tests#128sebastianrowan wants to merge 19 commits intoUSACE:reconstruction-lifecyclefrom
Conversation
HenryGeorgist
left a comment
HenryGeorgist
left a comment
There was a problem hiding this comment.
This was great. im asking you to remove files from the commit history that are not directly needed for the tests. i think that there are about 12 files currently included in this, we probably need only about 3 or 4 files total for this to be complete. lets make sure we only are changing those (to minimize our chances for accidentally checking in something we dont want to change)
structures/structures_test.go
Outdated
| t2 := time.Date(1984, time.Month(1), 11, 0, 0, 0, 0, time.UTC) // reconstruction from event 1 should be 50% complete at this time | ||
| d2.SetArrivalTime(t2) | ||
|
|
||
| events := []hazards.HazardEvent{d1, d2} |
| if err != nil { | ||
| panic(err) | ||
| } | ||
| fmt.Printf("Damage was %3.2f. Expected: %3.2f\n", dmgout, expectedDmgs[idx]) |
|
@HenryGeorgist I think I have removed all the edits to files that aren't involved in this PR. |
|
@HenryGeorgist this implementation now keeps track of structure/content values and the overall damage level throughout the series of events. When a new event arrives, we first calculate how much of the total damage (not just from the previous event) has been repaired since the last event ended. Then we calculate loss by applying the damage function to the adjusted structure value. Finally we calculate reconstruction time and completion date based on the total current damage state not just the loss from the current event. |
|
|
||
| events := []hazards.ArrivalDepthandDurationEvent{d5, d1, d2, d3, d4} | ||
|
|
||
| addMulti := &hazards.ArrivalDepthandDurationEventMulti{Events: events} //need to use the pointer reference because methods on MultiHazardEvent require pointers |
There was a problem hiding this comment.
@HenryGeorgist will this be a problem? The Increment() and ResetIndex() methods on the MultiHazardEvent have to use pointers to be able to modify the index value of the struct.
trying to send addMulti to Compute() without the & means it doesn't satisfy the MultiHazardEvent interface.
There was a problem hiding this comment.
either an instance will implement all the methods or the pointer will implement all the methods. So if you have to have a pointer on one then you have to have a pointer on all methods. An alternative: func (h ArrivalDepthAndDurationEvent) Index() ArrivalDepthAndDurationEvent where the interface would have to be Index() MultiHazard
| panic(err) | ||
| } | ||
| //compute consequences. | ||
| StreamAbstract(dfr, nsp, w) |
There was a problem hiding this comment.
ismt it cool that we get such robustly different behavior out of streamabstract when we leverage the abstractions!
hazardproviders/json_multi_hazard.go
Outdated
| process HazardFunction | ||
| } | ||
|
|
||
| type ADDProperties struct { // will try to not use this |
There was a problem hiding this comment.
my first response was why not just use a csv... but json is fine it really doesnt matter. You could also just randomly generate this information or hard code in the list of events for this level of testing we are currenly working on.
| "github.com/USACE/go-consequences/hazards" | ||
| ) | ||
|
|
||
| type jsonArrivalDepthDurationMultiHazardProvider struct { |
There was a problem hiding this comment.
using cogs at this point is overkill
| // We'll probably want to do something different here. | ||
| // Probably allow user to define study bbox with a | ||
| // shapefile/geojson/directly entering bbox coords | ||
| return j.depthCRs[0].GetBoundingBox() |
There was a problem hiding this comment.
this is great for now
| } | ||
|
|
||
| func (h ArrivalDepthandDurationEventMulti) Sort() { // ensure the hazard events are in order of arrival time | ||
| sort.Sort(h) |
There was a problem hiding this comment.
im not sure i like this. i could get comfortable with it. but my first inclination is that we would need to check if the data is not sorted and if it is not reject the dataset and throw an error on the load of the provider (or soemthing) so that the compute doesnt continue with bad data.
There was a problem hiding this comment.
Yeah. Maybe there are use cases where it would be beneficial for us to sort the data, but I agree that we should just force users to supply properly formatted datasets. If we change the order of the events silently, the user will not get the output they expect.
Another consideration for future work is that if we are processing multiple series of events generated from a Monte Carlo process as a single input, when the list of events from one lifecycle ends and the next begins, the dates of events will be out of order, and sorting this way would mix up the lifecycles.
| if addm.Index() != 0 { | ||
| t.Errorf("Index not reset") | ||
| } | ||
| // check that we can read the Parameters for the events |
There was a problem hiding this comment.
this is great. maybe we could check values since we have the expected array
There was a problem hiding this comment.
This is handled on line 162. This was an additional check to confirm the functionality of Has()
| return nil | ||
| } | ||
|
|
||
| type MultiHazardEvent interface { |
There was a problem hiding this comment.
love it. we may want to step through each intrface and determine its criticality. we may have more overall methods than necessary
2 participants
This PR adds two functions to the
structuresmodule.computeConsequencesWithReconstruction()calculates the number of days to complete construction on a damaged structure and includes this in the resultTestComputeConsequencesWithReconstruction()computeConsequencesMulti()calculates the number of days and final date to complete reconstruction for an array of hazard events.TestComputeConsequencesMulti()